Preserve gRPC channel recreation when externalized payloads are enabled - #797

Summary

Enabling UseExternalizedPayloads (the AzureBlobPayloads large-payload extension) silently disabled gRPC channel recreation on both the worker and the client. This is a shipped bug on main, introduced by #468 (ae1433aa).

User-visible impact

Channel recreation is on by default (ChannelRecreateFailureThreshold = 5 in both GrpcDurableTaskWorkerOptions and GrpcDurableTaskClientOptions). Its whole purpose, per SetChannelRecreator's own doc comment, is to recover from "repeated connect failures (e.g., because the backend was replaced and the existing channel is wedged on a half-open HTTP/2 connection)".

With externalized payloads enabled, that recovery was gone. On backend scale-out, upgrade, or node replacement, the worker wedges on a half-open HTTP/2 connection, stops processing work, and never recovers until the process is manually restarted.

Root cause

IConfigureOptions runs before IPostConfigureOptions.

1. DTS's ConfigureGrpcChannel (an IConfigureOptions) sets both options.Channel (DurableTaskSchedulerWorkerExtensions.cs:176) and options.SetChannelRecreator(...) (:185).
2. The AzureBlobPayloads PostConfigure then ran, moved the channel onto an intercepted CallInvoker, and nulled options.Channel.

It had to null Channel, because the core contract documents that "Channel ... will supersede CallInvoker" (GrpcDurableTaskClientOptions.cs:17) — leaving Channel set would make core build a raw invoker and bypass the interceptor entirely.

The consequence was that every recreation path died:

Probe output against the shipped code (DTS-shaped worker, IConfigureOptions setting Channel + recreator, then optional UseExternalizedPayloads):

A. WITHOUT externalized payloads: Channel=set  CallInvoker=null  path1 REACHABLE -> self-heal AVAILABLE
B. WITH    externalized payloads: Channel=null CallInvoker=set   path1 DEAD, path2 DEAD -> self-heal LOST

Why the simpler alternatives don't work

1. Just don't null the Channel. Core then takes the Channel branch and builds a raw invoker, so the interceptor is dropped at startup and large payloads break outright.
2. Flip core precedence to prefer CallInvoker. Directly contradicts the documented public contract at GrpcDurableTaskClientOptions.cs:17 and changes behavior for every consumer, not just this extension.
3. Have the extension wrap the recreator. Impossible from outside core: after recreating, the worker built the new invoker with a bare newChannel.CreateCallInvoker() with no extension hook, so the interceptor would be lost on every recreate anyway.

The fix: an internal CallInvokerDecorator hook

Stop mutating Channel/CallInvoker from the extension. Instead the extension registers a decorator, and core applies it at every point a CallInvoker is produced — including after a channel recreate. Channel stays set (so recreation works) and the interceptor is always applied (so large payloads work).

This mirrors the existing SetChannelRecreator idiom exactly (naming, placement, accessibility, doc-comment style).

• GrpcDurableTaskClientOptions.InternalOptions / GrpcDurableTaskWorkerOptions.InternalOptions: new Func<CallInvoker, CallInvoker>? CallInvokerDecorator.
• Both InternalOptionsExtensions: new SetCallInvokerDecorator(...) plus a public ApplyCallInvokerDecorator(options, invoker) helper (the extension is a separate assembly and cannot reach InternalOptions directly).
• GrpcDurableTaskClient.GetCallInvoker → renamed to GetCallInvokerCore, with a thin GetCallInvoker wrapper that applies the decorator once. The decorator is applied outside ChannelRecreatingCallInvoker, so the wrapper's internal channel swaps stay transparent to the interceptor.
• GrpcDurableTaskWorker.GetCallInvoker → same rename + wrap.
• GrpcDurableTaskWorker.TryRecreateChannelAsync applies the decorator to the new invoker at both recreate sites. This is the crux — without it the interceptor would be silently lost on every recreate, which would be worse than the original bug.
• Both AzureBlobPayloads DI extensions: the whole if (Channel) / else if (CallInvoker) / else throw block is replaced with a single SetCallInvokerDecorator(inv => inv.Intercept(new AzureBlobPayloadsSideCarInterceptor(store, opts))). Channel is no longer nulled. The worker still adds P.WorkerCapability.LargePayloads.

Design invariant: purely additive to core. With no decorator set, behavior is byte-for-byte identical to today. This is asserted by tests on both the worker and the client.

Bonus bug fixed

Deleting the else { throw } fixes a second, independent defect: today UseGrpc("http://localhost:4001") — the Address-only form, the most common raw-gRPC worker setup — throws ArgumentException and is completely unusable with externalized payloads. Only UseGrpc(o => o.CallInvoker = ...) worked. With the decorator, core creates its channel from Address and the decorator still applies. One change, two bugs.

Tests

New coverage (all four suites green):

• Extension regression tests (ExternalizedPayloadsCallInvokerDecoratorTests, 8 tests) — worker and client Channel is no longer nulled; Address-only no longer throws; the external-CallInvoker path is no longer mutated; LargePayloads capability is still announced; and two end-to-end interception tests that drive a real CreateInstanceRequest through the decorated invoker and assert the payload was actually uploaded to the store and replaced with a token. (There was no pre-existing interception test.)
• Worker core (GrpcDurableTaskWorkerCallInvokerDecoratorTests, 4 tests) — decorator applied to the initial invoker; not applied when unset; and applied to the invoker produced after a channel recreate on both recreate paths (reached via reflection on TryRecreateChannelAsync).
• Client core (GrpcDurableTaskClientCallInvokerDecoratorTests, 5 tests) — decorator applied on the Channel, external-CallInvoker, and Address paths; applied outside ChannelRecreatingCallInvoker; and not applied when unset.
• Options contract — CallInvokerDecorator defaults to null, SetCallInvokerDecorator(null) throws, ApplyCallInvokerDecorator is identity without a decorator.

Regression evidence

The extension regression tests were written first and observed failing on the base commit (ab4be125) — 2 failing with Expected options.Channel to refer to ... but found <null>, 2 with the ArgumentException thrown from DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:78. All pass after the fix.

Verification

dotnet build Microsoft.DurableTask.sln --no-incremental
  base ab4be125 : 0 Error(s), 340 Warning(s)
  this branch   : 0 Error(s), 340 Warning(s)
  normalized warning-set Compare-Object: 226 vs 226 -> IDENTICAL (0 differences)

AzureBlobPayloads.Tests  Passed!  Failed: 0, Passed:  15
Client.Tests             Passed!  Failed: 0, Passed:  43
Client.Grpc.Tests        Passed!  Failed: 0, Passed:  56
Worker.Grpc.Tests        Passed!  Failed: 0, Passed: 159

Notes

• Independent of Add large-payload blob auto-purge (opt-in singleton job, worker/SDK side) #758 — branched directly off main, does not touch that branch.
• No public API change and no proto change. SetCallInvokerDecorator / ApplyCallInvokerDecorator live in the .Internal namespace alongside SetChannelRecreator and carry the standard "not subject to the same compatibility standards" remarks.